[velero] Adhere chart to semver #569

qiuming-best · 2024-04-19T09:02:37Z

Special notes for your reviewer:

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

DCO signed
Chart Version bumped
Variables are documented in the values.yaml or README.md
Title of the PR starts with chart name (e.g. [velero])

Fix #554

Add helm release instruction document.
Modify release workflow, make any commit merged into main or velero-helm-charts-v* trigger helm chart release.
Modify lint-test, trigger test if the current submit different from the target branch

Signed-off-by: Ming Qiu <[email protected]>

siegenthalerroger · 2024-04-19T10:20:56Z

looks good to me :)

jenting

@qiuming-best Thank you for coordinate the topic and comes out the idea and documentation. I have another ideas on this one, please take a look of my comments.

jenting · 2024-04-21T13:08:13Z

RELEASE-INSTRUCT.md

+#### Guidelines
+To comply with the [Semantic Versioning (semver)](https://semver.org/#summary) rule, follow these instructions:
+- **Velero Release:**
+  - For each Velero minor version release, increase the major version of the Helm charts. For example, if Velero v1.13.0 is released, the Helm charts version becomes 6.0.0; for Velero v1.14.0, it becomes 7.0.0.


Personally, I don't think the helm chart itself follows the semver because the Velero bumps minor version (functionality change) but the corresponding helm chart changes the major version (breaking changes).

We could consider maintains the velero helm chart and Velero itself with this format . This makes the helm chart follows the semver as well, also we can easily check the helm chart upstream Velero version.

@jenting
I do not quite understand the format :
The upstream charts follow this versioning: 1.0.#+upX.Y.Z
Could you please give more details explanation?

Our main goal for this PR about versioning is to solve the conflicts in chart versions problem
Not sure the format you mentioned could avoid the conflicts in chart versions problem?

If it could avoid the conflicts, we also need to plan the helm chart version reasonably related format in 1.0.#+upX.Y.Z ?

@jenting the reasoning behind the chart requiring a major bump for every minor velero bump is because velero itself doesn't follow semver. There are often changes in minor velero updates that require changes to be made to the deployment. To a certain extent the helm chart could catch these and make them non-breaking, however the decision here would be to forego that effort and just admit that the helm chart just "moves fast and breaks things" in a sense.

Having looked at the rancher example, they actually have the same "ruleset" in a sense. The helm major version is always bumped for a minor rancher release (the 100.x.y examples), just with an additional +up....

Personally I don't think the additional +up is necessary, that's what the appVersion field in the Chart.yaml is for, and it's what we use to track the upstream version in our deployments.

What if the Velero bump the major release? How should we tackle in the Velero helm chart?

My thought was

if the Velero bump patch/minor/major version, the helm chart itself should bump patch/minor/major version.

if the helm chart itself have to fix/feature/breaking change, bump the helm chart version ..-. For example: current version is 6.1.0-v1.13.0

The helm chart bug fix would be 6.1.1-v1.13.0

The helm chart feature enhance would be 6.2.0-v1.13.0

The helm chart breaking change would be 7.0.0-v1.13.0

The velero bug fix would be 6.1.1-v1.13.1

The velero feature enhance would be 6.2.0-v1.14.0

The velero bbreaking change would be 7.0.0-v2.0.0

To the first question: just do a major bump as you'd expect.

To your suggestion: While that sounds fine, the issue raised was that the helm chart can sometimes manage to update to new velero minor versions without a breaking change, but sometimes it does. IMO, while the originally suggested change veers on the safe side (always doing major bumps for minor velero updates, just incase they have a breaking change), you could cover it just by making sure it major bumps if a breaking change is required.

Understood.

Adhere chart to semver

c7ceea8

Signed-off-by: Ming Qiu <[email protected]>

github-actions bot requested review from jenting and ywk253100 April 19, 2024 09:02

github-actions bot assigned qiuming-best Apr 19, 2024

qiuming-best requested a review from reasonerjt April 19, 2024 09:08

jenting reviewed Apr 21, 2024

View reviewed changes

jenting approved these changes Apr 23, 2024

View reviewed changes

ywk253100 approved these changes May 10, 2024

View reviewed changes

jenting merged commit 813909d into vmware-tanzu:main May 11, 2024
15 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[velero] Adhere chart to semver #569

[velero] Adhere chart to semver #569

qiuming-best commented Apr 19, 2024

siegenthalerroger commented Apr 19, 2024

jenting left a comment

jenting Apr 21, 2024

qiuming-best Apr 22, 2024 •

edited

Loading

siegenthalerroger Apr 22, 2024

siegenthalerroger Apr 22, 2024

jenting Apr 22, 2024

siegenthalerroger Apr 22, 2024

jenting Apr 23, 2024

[velero] Adhere chart to semver #569

[velero] Adhere chart to semver #569

Conversation

qiuming-best commented Apr 19, 2024

Special notes for your reviewer:

Checklist

siegenthalerroger commented Apr 19, 2024

jenting left a comment

Choose a reason for hiding this comment

jenting Apr 21, 2024

Choose a reason for hiding this comment

qiuming-best Apr 22, 2024 • edited Loading

Choose a reason for hiding this comment

siegenthalerroger Apr 22, 2024

Choose a reason for hiding this comment

siegenthalerroger Apr 22, 2024

Choose a reason for hiding this comment

jenting Apr 22, 2024

Choose a reason for hiding this comment

siegenthalerroger Apr 22, 2024

Choose a reason for hiding this comment

jenting Apr 23, 2024

Choose a reason for hiding this comment

qiuming-best Apr 22, 2024 •

edited

Loading